allow defining ipFamilyPolicy for external service #362
Conversation
nadiamoe
force-pushed
the
external-service-family
branch
2 times, most recently
from
b52e938 to
bb6fbac
Compare
nadiamoe
force-pushed
the
external-service-family
branch
from
bb6fbac to
da26800
Compare
|
CI/CD fails for reasons unknown to me, but I think the PR itself should be okay. |
| @@ -19,6 +19,9 @@ spec: | |||
| {{- with .Values.front.externalService }} | |||
| type: {{ .type | default "ClusterIP" }} | |||
| externalTrafficPolicy: {{ .externalTrafficPolicy | default "Local" }} | |||
| {{- with .ipFamilyPolicy }} | |||
There was a problem hiding this comment.
Why use with here and not if?
It would seem that it's not certain that ipFamilyPolicy exists.
with assumes your variable is present: https://helm.sh/docs/chart_template_guide/control_structures/#modifying-scope-using-with
There was a problem hiding this comment.
With and if should behave the same if the variable is absent (not yielding anything), but with only makes you write the name of the field (ipFamilyPolicy) once. So the chance to make a typo is reduced by 50% 😁
There was a problem hiding this comment.
The "typo chance" is not a joke, I think the vast majority of helm chart bugs I've seen, fixed, and introduced come from copypasting snippets and forget to change part of them.
|
@fastlorenzo this looks good to me, can we get it merged? |
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
Unstale |
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
Unstale |
|
Hey folks, this has been lingering for a long while now. Is there anything I can do to help get this merged? |
|
Hi @roobre I'm in contact with the maintainer to see how we can spped things up here. |
For dual stack (IPv4 and IPv6) clusters, services need to be explicitly opted-in into having two addresses. This is done through the
ipFamilyPolicyfield, as documented in https://kubernetes.io/docs/concepts/services-networking/dual-stack/#services.I've wired the templates in a way where if this field is not present, nothing is rendered. This preserves the usual behavior of not specifying this value altogether for services.